Skip to content

Fix memory leak in dependency nodes#26

Merged
akochurov merged 1 commit into
maxifier:masterfrom
akochurov:fix-memory-leak
Jan 29, 2016
Merged

Fix memory leak in dependency nodes#26
akochurov merged 1 commit into
maxifier:masterfrom
akochurov:fix-memory-leak

Conversation

@akochurov

Copy link
Copy Markdown
Contributor

Dependency nodes should regularly cleanup their dependent nodes references in order to remove the ones that point to GC'ed dependents

Dependency nodes should regularly cleanup their dependent nodes references
  in order to remove the ones that point to GC'ed dependents
@akochurov

Copy link
Copy Markdown
Contributor Author

@xrcat please have a look

@akochurov

Copy link
Copy Markdown
Contributor Author

@not-azat please have a look too.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why it is not in AbstractCache?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two different concepts:

  • Cache - takes care of storing the data and accessing it;
  • DependencyNode - takes care about dependencies.

There are few implementations XXXInlineDependencyCache that implement both Cache and DependencyNode. This reduces the memory overhead of caching in some simple cases.

But cleanupIfNeeded stuff is not part of Cache contract, so AbstractCache has nothing to do with that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be create helper class? Too much code duplicating.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is a template, others are autogenerated from this template.

It doesn't seem reasonable to create some kind of helper for a single method or add one more class to the hierarchy.

Code generation is bad, I know. But there's no other way to have high performance with primitives in Java.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@klya

klya commented Jan 28, 2016

Copy link
Copy Markdown
Member

As I've mentioned custom HashSet looks like best alternative.

@akochurov

Copy link
Copy Markdown
Contributor Author

Ok, we've decided to merge it as-is.

akochurov added a commit that referenced this pull request Jan 29, 2016
Fix memory leak in dependency nodes
@akochurov akochurov merged commit 14c118b into maxifier:master Jan 29, 2016
@akochurov akochurov deleted the fix-memory-leak branch January 29, 2016 10:33
@akochurov akochurov self-assigned this Jan 29, 2016
@akochurov akochurov added this to the 2.6.3 milestone Jan 29, 2016
@akochurov akochurov added the bug label Feb 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants